Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Track parser position for error reporting #76

Closed
wants to merge 1 commit into from

Conversation

joelreymont
Copy link
Contributor

No span tracking is being done, just position tracking for error reporting.

@joelreymont
Copy link
Contributor Author

Why do tests keep failing in CI?

@Hejsil
Copy link
Owner

Hejsil commented Nov 7, 2024

Why do tests keep failing in CI?

No idea. I'll take a proper look this weekend 🙏

@Hejsil
Copy link
Owner

Hejsil commented Nov 11, 2024

Why do tests keep failing in CI?

You have these lines:

var res: Res = undefined;
var pos: usize = 0;
res.ok.rest = str;

This is illegal behavior. You're initializing Res to undefined and then accessing through the union with res.ok. res.ok is not the active field. Nothing is the active field.

I don't see why you need to initialize a Res right here. Just keep track of Res.Ok

var res: Res.Ok = undefined;
var pos: usize = 0;
res.rest = str;

@joelreymont
Copy link
Contributor Author

I wonder if this is a compiler regression because my 2 Macs have different versions of the Zig master and they haven't caught this.

@joelreymont
Copy link
Contributor Author

Is anything else missing here?

Can you accept the PR?

@Hejsil
Copy link
Owner

Hejsil commented Nov 12, 2024

Is anything else missing here?

Can you accept the PR?

There are some changes I want to make, which have ended up taking a bit. Hopefully this will be merged in some modified form within this week

@joelreymont
Copy link
Contributor Author

What changes are you planning?

@Hejsil
Copy link
Owner

Hejsil commented Nov 12, 2024

My plan is to make Result this:

pub fn Result(comptime T: type) type {
    return struct {
        index: usize,
        value: union(enum) {
            ok: T,
            err,
        },

        pub fn ok(index: usize, value: T) @This() {
            return .{ .index = index, .value = .{ .ok = value } };
        }

        pub fn err(index: usize) @This() {
            return .{ .index = index, .value = .err };
        }
    };
}

Which results in a lot of small changes. There where also some bugs I'm ironing out which hopefully should yield even better error positions (in the json example, the error position is now rarely 0)

@Hejsil
Copy link
Owner

Hejsil commented Nov 15, 2024

Thanks for the initial implementation. It has been merged with my modifications: 7100cb3

@Hejsil Hejsil closed this Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants